Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client/fingerprint: detect unloaded dynamic bridge kernel module #9299

Merged
merged 2 commits into from
Nov 10, 2020

Conversation

shoenig
Copy link
Member

@shoenig shoenig commented Nov 9, 2020

In Nomad v0.12.0, the client added additional fingerprinting around the
presense of the bridge kernel module. The fingerprinter only checked in
/proc/modules which is a list of loaded modules. In some cases, the
bridge kernel module is builtin rather than dynamically loaded. The fix
for that case is in #8721. However we were still missing the case where
the bridge module is dynamically loaded, but not yet loaded during the
startup of the Nomad agent. In this case the fingerprinter would believe
the bridge module was unavailable when really it gets loaded on demand.

This PR now has the fingerprinter scan the kernel module dependency file,
which will contain an entry for the bridge module even if it is not yet
loaded.

In summary, the client now looks for the bridge kernel module in

  • /proc/modules
  • /lib/modules/<kernel>/modules.builtin
  • /lib/modules/<kernel>/modules.dep

Closes #8423

In Nomad v0.12.0, the client added additional fingerprinting around the
presense of the bridge kernel module. The fingerprinter only checked in
`/proc/modules` which is a list of loaded modules. In some cases, the
bridge kernel module is builtin rather than dynamically loaded. The fix
for that case is in #8721. However we were still missing the case where
the bridge module is dynamically loaded, but not yet loaded during the
startup of the Nomad agent. In this case the fingerprinter would believe
the bridge module was unavailable when really it gets loaded on demand.

This PR now has the fingerprinter scan the kernel module dependency file,
which will contain an entry for the bridge module even if it is not yet
loaded.

In summary, the client now looks for the bridge kernel module in
 - /proc/modules
 - /lib/modules/<kernel>/modules.builtin
 - /lib/modules/<kernel>/modules.dep

Closes #8423
@shoenig
Copy link
Member Author

shoenig commented Nov 9, 2020

This is trying hard to avoid an exec modinfo; open to weighing the pros and cons.

Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

I took a look at the source of modprobe and the output of strace while searching for a bogus module and it looks like this should work.

One small caveat is that modprobe has configuration values that can tell it to look elsewhere; in practice I suspect this isn't something that's going to impact bridge but we should at least have that in the back of our mind if we get a report about someone's exotic setup later down the road.

@shoenig shoenig merged commit 76328b9 into master Nov 10, 2020
@shoenig shoenig deleted the b-detect-unloaded-kmod branch November 10, 2020 14:24
@apollo13
Copy link
Contributor

apollo13 commented Jan 11, 2021

Hi @shoenig, on which systems did you check this PR? @jeroentbt ran over this in Gitter and told me that he uses CentOS. When I checked against my CentOS 7 boxes they do have the following in modules.dep:

[root@mon01 ~]# cat /lib/modules/3.10.0-1160.11.1.el7.x86_64/modules.dep|grep bridge.ko
kernel/drivers/net/ethernet/mellanox/mlxsw/mlxsw_spectrum.ko.xz: kernel/drivers/net/ethernet/mellanox/mlxfw/mlxfw.ko.xz kernel/net/ipv6/ip6_tunnel.ko.xz kernel/net/ipv6/tunnel6.ko.xz kernel/net/psample/psample.ko.xz kernel/lib/parman.ko.xz kernel/net/bridge/bridge.ko.xz kernel/net/802/stp.ko.xz kernel/net/llc/llc.ko.xz kernel/drivers/net/ethernet/mellanox/mlxsw/mlxsw_pci.ko.xz kernel/drivers/net/ethernet/mellanox/mlxsw/mlxsw_core.ko.xz kernel/net/core/devlink.ko.xz
kernel/drivers/media/pci/ddbridge/ddbridge.ko.xz: kernel/drivers/media/dvb-core/dvb-core.ko.xz
kernel/net/netfilter/xt_physdev.ko.xz: kernel/net/bridge/br_netfilter.ko.xz kernel/net/bridge/bridge.ko.xz kernel/net/802/stp.ko.xz kernel/net/llc/llc.ko.xz
kernel/net/bridge/netfilter/nf_tables_bridge.ko.xz: kernel/net/netfilter/nf_tables.ko.xz kernel/net/netfilter/nfnetlink.ko.xz
kernel/net/bridge/netfilter/nft_meta_bridge.ko.xz: kernel/net/netfilter/nft_meta.ko.xz kernel/net/netfilter/nf_tables.ko.xz kernel/net/netfilter/nfnetlink.ko.xz
kernel/net/bridge/netfilter/nft_reject_bridge.ko.xz: kernel/net/bridge/netfilter/nf_tables_bridge.ko.xz kernel/net/ipv4/netfilter/nf_reject_ipv4.ko.xz kernel/net/ipv6/netfilter/nf_reject_ipv6.ko.xz kernel/net/netfilter/nft_reject.ko.xz kernel/net/netfilter/nf_tables.ko.xz kernel/net/netfilter/nfnetlink.ko.xz kernel/net/bridge/bridge.ko.xz kernel/net/802/stp.ko.xz kernel/net/llc/llc.ko.xz
kernel/net/bridge/netfilter/nf_log_bridge.ko.xz:
kernel/net/bridge/netfilter/ebtable_broute.ko.xz: kernel/net/bridge/netfilter/ebtables.ko.xz kernel/net/bridge/bridge.ko.xz kernel/net/802/stp.ko.xz kernel/net/llc/llc.ko.xz
kernel/net/bridge/bridge.ko.xz: kernel/net/802/stp.ko.xz kernel/net/llc/llc.ko.xz
kernel/net/bridge/br_netfilter.ko.xz: kernel/net/bridge/bridge.ko.xz kernel/net/802/stp.ko.xz kernel/net/llc/llc.ko.xz

would it be possible to include (\.xz)? in the regex in https://github.com/hashicorp/nomad/pull/9299/files#diff-25979a0adb76fadc59ae57997b3d3e70ac415168d760bd3e6d95b8db1cca7cc6R19 ?

@shoenig
Copy link
Member Author

shoenig commented Jan 12, 2021

Thanks for pointing that out @apollo13, I think fixing the regexp makes sense. I created #9776 to track that. Would you like to open a PR? Otherwise I can get to it a bit later

@github-actions
Copy link

github-actions bot commented Dec 4, 2022

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Connect jobs failing in 12.0
3 participants